-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Dialog to select proposal/directory when starting GUI #9
Conversation
0825234
to
6585dc3
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Things we discussed:
- Adding tests
- Open dialog from menu entry
- Creating a backend from the dialog if one doesn't exist
Co-authored-by: James Wrigley <[email protected]>
Thanks @JamesWrigley for the review. I've made all the changes we discussed now; do you want to take another look overt the latest changes? The main change, other than the tests, is to the code for creating a DAMNIT DB and starting the backend if they don't already exist. |
Hmmm, now I'm getting crashes when testing this on Maxwell and my local machine 🤔 Both with selecting by proposal and directory:
|
Gah, threads. I saw those errors while running the tests and fixed them there, but then didn't try again with the actual application. I'll take a look. |
OK, I think I've finally got the application, the local tests and the CI all working 😅 🤞 . |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't have quite enough time to test it myself, but I trust you and CI 😁 LGTM!
(P.S. I might also suggest squashing some of the commits before merging, but I leave that up to you)
I haven't tried to do any refactoring to take full advantage of this yet.